Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performer select calculated ages #5110

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

dogwithakeyboard
Copy link
Contributor

@dogwithakeyboard dogwithakeyboard commented Aug 2, 2024

Following on from #5076 this shows the age of the performer in the select based on the scene/image/gallery date, when one exists.

The width of the performer select in the tagger view has also been slightly increased.

perfage3

@DogmaDragon
Copy link
Collaborator

Censored the image for safety reasons.

@echo6ix
Copy link
Contributor

echo6ix commented Aug 3, 2024

From what I can tell it looks like the $age value is being inserted into the CSS birthdate selector. The age value should get its own CSS selector.

<span className="performer-select-age">{$age}</span>

@stashapp stashapp deleted a comment Aug 4, 2024
@stashapp stashapp deleted a comment Aug 4, 2024
@echo6ix
Copy link
Contributor

echo6ix commented Aug 7, 2024

@kermieisinthehouse @dogwithakeyboard @WithoutPants

image

This is very nitpicky, and I do not want to stifle this PR, but the proposed locale string {age} on this date comes directly after the the birthdate (yyyy-mm-dd) value. See above image.

To you and I, we know which date is being referenced, but on it's face as far as clear and unambiguous language is concerned, this is really awkward phrasing as someone who's not familiar with the app could misconstrue what "on this date" is referencing, especially being juxtaposed to the birthdate .

To remove ambiguity I would suggest {age} {years old} on scene date and {age} {years old} on image date respectively. It goes back to referencing the context, but in a way that does not imply they were in the content.

Does that make sense?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants